Skip to content

[My Store] Add support for conditionally storing site summary stats #8421

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 7 commits into from
Dec 15, 2022
Merged

[My Store] Add support for conditionally storing site summary stats #8421

merged 7 commits into from
Dec 15, 2022

Conversation

@rachelmcr
Copy link
Contributor

@rachelmcr rachelmcr commented Dec 15, 2022

Part of #8173
⚠️ Depends on #8420 ⚠️

Description

This PR adds support in the Yosemite layer for conditionally storing SiteSummaryStats fetched from remote:

  • Adds support for retrieving SiteSummaryStats from storage.
  • Adds a saveInStorage parameter to StatsActionV4.retrieveSiteSummaryStats to determine if stats should be stored.
  • Updates StatsStoreV4 to support conditionally upserting stats fetched from remote.

Note: For now we only store SiteSummaryStats that are fetched for a single period. The My Store dashboard only needs summary stats for a single period (day, week, month, year) and stats fetched for multiple periods don't need to be stored (used only in the Analytics Hub, for quarter periods).

Testing instructions

  1. Build and run the app.
  2. Tap "See more" to open Analytics.
  3. Confirm the Sessions card loads as expected for different time ranges.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@rachelmcr rachelmcr added the feature: stats Related to stats, including Top Performers. label Dec 15, 2022
@rachelmcr rachelmcr added this to the 11.7 milestone Dec 15, 2022
@rachelmcr rachelmcr force-pushed the issue/8173-sitesummarystate-yosemite branch from 8c5cfd2 to 4fcd0eb Compare December 15, 2022 15:04
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Dec 15, 2022

You can test the changes from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr8421-4fcd0eb on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@rachelmcr rachelmcr marked this pull request as ready for review December 15, 2022 15:52
Base automatically changed from issue/8173-sitesummarystats-storage to trunk December 15, 2022 16:14
@peril-woocommerce
Copy link

peril-woocommerce bot commented Dec 15, 2022

Warnings
⚠️ This PR is assigned to a milestone which is closing in less than 2 days Please, make sure to get it merged by then or assign it to a later expiring milestone

Generated by 🚫 dangerJS

@Ecarrion Ecarrion self-assigned this Dec 15, 2022
Ecarrion
Ecarrion approved these changes Dec 15, 2022
View reviewed changes
Copy link
Contributor

@Ecarrion Ecarrion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good! But now that I think a bit more about the WHY of these changes I think I'm not understanding something 😅

As far as I can tell, this PR is adding the ability to retrieveSiteSummaryStats to store results on the storage layer, right?

But in the PR you say that this is needed for the Dashboard Stats, does that mean that we will be adding this information on the dashboard?

Or did I miss-understood this 😅 ?

@rachelmcr
Copy link
Contributor Author

rachelmcr commented Dec 15, 2022

But in the PR you say that this is needed for the Dashboard Stats, does that mean that we will be adding this information on the dashboard?

The information is already in the dashboard ("Visitors" and "Conversion rate"), but it's showing inaccurate data:

  • Right now, we're computing a site's total visitor count by adding together the visitor count for each of the intervals within a given period (from SiteVisitStats data).
  • What we should do in the dashboard —and we're doing already in Analytics Hub — is get the total visitor count for the entire period (from SiteSummaryStats data).

The first approach is inaccurate because visitors are uniques, so the same visitor should only be counted once even if they visit multiple times during the entire period.

Since we're using the accurate method in Analytics Hub, I'm fixing it in the Dashboard stats to make sure there aren't inconsistencies between the stats shown on the two screens.

@rachelmcr rachelmcr merged commit e9c7c10 into trunk Dec 15, 2022
@rachelmcr rachelmcr deleted the issue/8173-sitesummarystate-yosemite branch December 15, 2022 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ealeksandrov ealeksandrov Awaiting requested review from ealeksandrov

@ThomazFB ThomazFB Awaiting requested review from ThomazFB

1 more reviewer

@Ecarrion Ecarrion Ecarrion approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@Ecarrion Ecarrion

Labels

feature: stats Related to stats, including Top Performers.

Projects

None yet

Milestone

11.7

Development

Successfully merging this pull request may close these issues.

4 participants

@rachelmcr @wpmobilebot @Ecarrion